Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prettify invoice details #1245

Merged
merged 10 commits into from
Sep 13, 2023
Merged

Prettify invoice details #1245

merged 10 commits into from
Sep 13, 2023

Conversation

Restioson
Copy link
Contributor

@Restioson Restioson commented Sep 7, 2023 •
edited
Loading

Fixes #872 by prettifying the invoice details modeled in #1111 and #1213

To do:

  • Show failed payments better
  • Show expired invoices better
  • Show pending invoices better
  • Add mempool link

@Restioson Restioson marked this pull request as draft September 7, 2023 12:25
@Restioson Restioson mentioned this pull request Sep 7, 2023
Merged
@Restioson
Copy link
Contributor Author

Restioson commented Sep 8, 2023 •
edited
Loading

Here follow some screenshots :)

Lightning payment (funding)

image

Lightning payment (outgoing)

image

Pending lightning transaction

image

Failed lightning transaction

image

Channel opening fee

image

Opening position

image

Matching fee

image

Closed position

image

Bitcoin transaction (incoming)

image

@bonomat
Copy link
Contributor

bonomat commented Sep 8, 2023

Looks nice, but.. .would it be possible to use this dialog which comes up from the bottom as shown in the design here?

@Restioson
Copy link
Contributor Author

Looks nice, but.. .would it be possible to use this dialog which comes up from the bottom as shown in the design #872 (comment)?

Definitely should be possible, I can just swap out the container :)

@Restioson
Copy link
Contributor Author

@bonomat: I've updated the screenshots

@Restioson Restioson marked this pull request as ready for review September 8, 2023 11:22
Restioson
Restioson commented Sep 8, 2023
View reviewed changes
mobile/lib/common/middle_ellipsised_text.dart
Copy link
Contributor Author

@Restioson Restioson Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This took me quite a while and I am still not quite happy with it, but it's good enough 😅

Intrinsic sizes and dialogues in flutter are weird

@holzeis
Copy link
Contributor

holzeis commented Sep 9, 2023 •
edited
Loading

Looks pretty nice already! 👍

Nit: I don't think we need to copy all rows. e.g. when, expiry, etc. 😅

Also the fonts seem pretty small can you make it bigger? And maybe have the content aligned to the right size with a bit space in between label and content? 😬

@bonomat
Copy link
Contributor

bonomat commented Sep 10, 2023

@bonomat: I've updated the screenshots

I think it looks better but I don't love it yet. I'm happy to play around with it if you don't have patience anymore.

1 similar comment
@bonomat
Copy link
Contributor

bonomat commented Sep 10, 2023

@bonomat: I've updated the screenshots

I think it looks better but I don't love it yet. I'm happy to play around with it if you don't have patience anymore.

@Restioson
Copy link
Contributor Author

@bonomat: sure, that's okay! If you have a clear vision for how it should look then go ahead :)

@Restioson
Copy link
Contributor Author

Note to self:

I think there is a lot of white in between and the text looks a bit small... but this is really just nitpicking [..]

Will try fix these for merge

@Restioson
Copy link
Contributor Author

New look:
image

bonomat
bonomat approved these changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@bonomat bonomat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, thanks for going over it once more.

@Restioson Restioson added this pull request to the merge queue Sep 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 13, 2023
@Restioson Restioson added this pull request to the merge queue Sep 13, 2023
Merged via the queue into main with commit 0ad05f3 Sep 13, 2023
7 checks passed
@Restioson Restioson deleted the pretty-invoice-details branch September 13, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bonomat bonomat bonomat approved these changes

@holzeis holzeis Awaiting requested review from holzeis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add transaction details to transactions in wallet history
3 participants
@Restioson @bonomat @holzeis